Skip to content

docs(approvals): unblock main — drop duplicated role warning, generalize approver literal#3120

Closed
os-zhuang wants to merge 1 commit into
mainfrom
fix/docs-role-word-approvals-lint
Closed

docs(approvals): unblock main — drop duplicated role warning, generalize approver literal#3120
os-zhuang wants to merge 1 commit into
mainfrom
fix/docs-role-word-approvals-lint

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Why

lint.yml has been red on main since #3113 — the Reserved-word ("role") docs ratchet step fails, so every open PR inherits it (e.g. #3116). Reproduces from a clean origin/main:

check-role-word: 1 problem(s)
  • content/docs/automation/approvals.mdx: role-word count grew 5 → 9. New occurrences are banned (ADR-0090 D3).

#3113 added four occurrences to content/docs/automation/approvals.mdx without ratcheting the baseline.

The decision: reword, not a baseline bump

Both obvious framings turned out to be wrong, so the evidence is worth stating.

"role is deprecated, so reword it away" — false. role is live and load-bearing:

  • packages/spec/src/automation/approval.zod.ts still enumerates 'role' alongside 'position' in ApproverType — they coexist and mean different things; neither is a migration half.
  • packages/spec/src/automation/approval.test.ts explicitly asserts 'role' parses.
  • expandRoleUsers resolves it against sys_member.role at runtime.
  • No deprecation marker anywhere on it.

role here is the better-auth org-membership tier (sys_member.role: owner/admin/member) — precisely ADR-0090 D3's single documented exception ("third-party schema we do not own"). #2738 (216fa9a) did not deprecate it; it added position beside it and documented role as "the membership tier it actually is".

"They're legitimate, so bump the baseline 5 → 9" — legitimate, but not necessary. Three of the four are a near-verbatim duplicate of the callout 80 lines earlier, and that earlier one is better (it names the approval-role-not-membership-tier lint rule). The fourth was an over-narrow API description. The baseline is for occurrences that can't be removed without losing accuracy — these can.

What changed

  • Removed the duplicated warning in the lifecycle Steps. Kept the fact that section actually needs (an approver entry resolving to nobody parks the run forever — the consequence) and cross-referenced the authoritative warning under 3. The approval node.
  • Generalized approverId: it was documented as accepting role:<r>. The runtime falls back to a generic `${a.type}:${a.value}` literal for every approver type (expandApprovers), and rest-server.ts only splits on commas — it never validates the prefix. <type>:<value> is the real contract; role:<r> described one arbitrary instance of it.

Net: same information, one authoritative home, no accuracy lost, and the reserved-word surface shrinks back to 5 instead of freezing 4 avoidable occurrences. The surviving 5 are the one better-auth boundary callout — deliberately untouched, since renaming a third-party identifier we don't own would make the docs wrong.

Verification

$ node scripts/check-role-word.mjs
check-role-word: OK (49 baselined file(s), no new occurrences).

scripts/role-word-baseline.json is unmodified — the diff is one file. The sibling lint.yml doc gates also pass locally:

  • check:doc-authoring199 files clean
  • check:release-notesOK

Notes for the reviewer

  • Docs-only → skip-changeset.
  • ⚠️ There is an uncommitted, unpushed draft of the same fix in a sibling worktree on branch fix/role-word-approvals (no PR opened). It reaches a similar conclusion but cross-references a broken anchor (#the-approval-node; the heading is ### 3. The approval node). If that agent is still active, close whichever of these lands second.

🤖 Generated with Claude Code

…teral (#3113 lint fix)

`lint.yml` has been red on main since #3113: the reserved-word ratchet
(ADR-0090 D3) caught content/docs/automation/approvals.mdx growing 5 → 9
occurrences of "role". Every open PR inherits the failure.

The four new occurrences are NOT a fifth legitimate boundary needing a
baseline bump — three of them are a near-verbatim duplicate of the callout
80 lines earlier, and the fourth is an over-narrow API description:

- The lifecycle Steps repeated the "type: 'role' is not a position" warning
  that "3. The approval node" already carries (and carries better — it names
  the `approval-role-not-membership-tier` lint rule). Kept the fact the
  lifecycle section actually needs (an unresolved approver list parks the run
  forever) and cross-referenced the authoritative warning instead.
- `approverId` was documented as accepting `role:<r>`. The runtime falls back
  to a generic `` `${a.type}:${a.value}` `` literal for EVERY approver type
  (approval-service.ts expandApprovers), and the REST layer only splits on
  commas — it never validates the prefix. `<type>:<value>` is the accurate
  contract; `role:<r>` described one arbitrary instance of it.

Net: same information, one authoritative home, no accuracy lost, and the
baseline stays at 5 rather than freezing 4 avoidable occurrences.

Deliberately NOT rewording the surviving 5: `role` there is the better-auth
org-membership tier (`sys_member.role`), which is D3's single documented
exception. It is live, not deprecated — spec `ApproverType` still enumerates
'role' alongside 'position' (approval.zod.ts, asserted in approval.test.ts)
and `expandRoleUsers` resolves it against `sys_member.role`. Renaming a
third-party identifier we do not own would make the docs wrong.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@os-zhuang os-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Jul 17, 2026
@vercel

vercel Bot commented Jul 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jul 17, 2026 12:14pm

Request Review

@os-zhuang

Copy link
Copy Markdown
Contributor Author

关闭:与 #3122 重复,且两者都是创可贴——只把文档里的重复表述删掉,没碰造成混淆的根因ApproverType.role 这个名字本身)。

改为按 #3133 的结论一次做对:把 role 改名成 ADR 早已规定的投影名 org_membership_level,文档随之不再需要写这个保留词,role-word 棘轮的红是改名的副产品而不是单独一轮修补。新 PR 会引用本 PR 与 #3122

@os-zhuang os-zhuang closed this Jul 17, 2026
@os-zhuang
os-zhuang deleted the fix/docs-role-word-approvals-lint branch July 17, 2026 12:22
xuyushun441-sys pushed a commit that referenced this pull request Jul 17, 2026
…l` (#3133)

`ApproverType.role` was the last platform surface projecting the word
ADR-0090 D3 reserved. Renaming it also unbreaks `lint.yml` on main: the
role-word ratchet has been red since #3113 (approvals.mdx 5 → 9), and the
docs stop needing the word once the type is spelled correctly.

D3's exception does not cover this enum. It protects better-auth's own
`sys_member.role` COLUMN — third-party schema we cannot rename. `ApproverType`
is ours: an authoring surface, i.e. the *projection*, which D3 says is spelled
`org_membership_level` and labelled "organization membership", never "role".

The sentence licensing the leak is itself false. ADR-0090 D3:203 claims
`sys_member.role` is "already relabelled `org_membership_level` in the platform
projection (ADR-0057 D7)" — but `org_membership_level` appeared nowhere in the
codebase (one comment in position.zod.ts), and ADR-0057 D7:335 lists that
relabel under "Deferred (evidence-gated, P4)". The projection never landed, so
the word reached authors.

The name manufactured a silent failure ("hotcrm class"): every sibling surface
renamed to `position` (`sys_role`, `ShareRecipientType.role`, `ctx.roles[]`),
so `{ type: 'role', value: 'sales_manager' }` reads as a position's legacy
spelling. It resolves against the membership tier, finds no member row, falls
back to an inert `role:sales_manager` literal, and the request waits forever.
Repo-wide, `type: 'role'` had ZERO real callers — only lint tests and the docs
warning that exists to undo the confusion the name creates.

- spec: `ApproverType` gains `org_membership_level`; `role` kept as a
  deprecated alias for one window so a published 15.x flow keeps loading.
  `DEPRECATED_APPROVER_TYPES` + `canonicalApproverType()` are the single
  source for the mapping (runtime and lint both read it).
- plugin-approvals: resolves on the canonical type, warns on the deprecated
  spelling, `expandRoleUsers` → `expandMembershipTierUsers`. The `type:value`
  fallback literal deliberately keeps the AUTHORED spelling — 15.x wrote
  `role:<v>` into `sys_approval_approver` / `pending_approvers`, and
  canonicalising it here would orphan every stored slot.
- lint: `approval-role-not-membership-tier` →
  `approval-approver-not-membership-tier` (the rule id carried the word too),
  plus `approval-approver-type-deprecated`. Mutually exclusive: a bad VALUE
  wins, because prescribing `org_membership_level` for a position name is
  wrong advice — the fix there is `position`.
- docs/skill/reference + role-word baseline ratcheted DOWN (approvals.mdx
  5 → 1, automation SKILL.md 3 → 1); api-surface snapshot regenerated
  (0 breaking, 2 added).

Studio still offers "Role" and its picker calls `client.list('role')` on a
metadata type D3 deleted — that picker is already dead, and the dropdown is
objectui's own hardcoded copy of this enum. Tracked as objectui follow-up in
degrades to free text (strictly better than a picker that lists nothing).

Closes #3133. Supersedes #3120 and #3122, which only deleted the duplicated
docs callout without touching the name that causes the confusion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
xuyushun441-sys pushed a commit that referenced this pull request Jul 17, 2026
…l` (#3133)

`ApproverType.role` was the last platform surface projecting the word
ADR-0090 D3 reserved. Renaming it also unbreaks `lint.yml` on main: the
role-word ratchet has been red since #3113 (approvals.mdx 5 → 9), and the
docs stop needing the word once the type is spelled correctly.

D3's exception does not cover this enum. It protects better-auth's own
`sys_member.role` COLUMN — third-party schema we cannot rename. `ApproverType`
is ours: an authoring surface, i.e. the *projection*, which D3 says is spelled
`org_membership_level` and labelled "organization membership", never "role".

The sentence licensing the leak is itself false. ADR-0090 D3:203 claims
`sys_member.role` is "already relabelled `org_membership_level` in the platform
projection (ADR-0057 D7)" — but `org_membership_level` appeared nowhere in the
codebase (one comment in position.zod.ts), and ADR-0057 D7:335 lists that
relabel under "Deferred (evidence-gated, P4)". The projection never landed, so
the word reached authors.

The name manufactured a silent failure ("hotcrm class"): every sibling surface
renamed to `position` (`sys_role`, `ShareRecipientType.role`, `ctx.roles[]`),
so `{ type: 'role', value: 'sales_manager' }` reads as a position's legacy
spelling. It resolves against the membership tier, finds no member row, falls
back to an inert `role:sales_manager` literal, and the request waits forever.
Repo-wide, `type: 'role'` had ZERO real callers — only lint tests and the docs
warning that exists to undo the confusion the name creates.

- spec: `ApproverType` gains `org_membership_level`; `role` kept as a
  deprecated alias for one window so a published 15.x flow keeps loading.
  `DEPRECATED_APPROVER_TYPES` + `canonicalApproverType()` are the single
  source for the mapping (runtime and lint both read it).
- plugin-approvals: resolves on the canonical type, warns on the deprecated
  spelling, `expandRoleUsers` → `expandMembershipTierUsers`. The `type:value`
  fallback literal deliberately keeps the AUTHORED spelling — 15.x wrote
  `role:<v>` into `sys_approval_approver` / `pending_approvers`, and
  canonicalising it here would orphan every stored slot.
- lint: `approval-role-not-membership-tier` →
  `approval-approver-not-membership-tier` (the rule id carried the word too),
  plus `approval-approver-type-deprecated`. Mutually exclusive: a bad VALUE
  wins, because prescribing `org_membership_level` for a position name is
  wrong advice — the fix there is `position`.
- docs/skill/reference + role-word baseline ratcheted DOWN (approvals.mdx
  5 → 1, automation SKILL.md 3 → 1); api-surface snapshot regenerated
  (0 breaking, 2 added).

Studio still offers "Role" and its picker calls `client.list('role')` on a
metadata type D3 deleted — that picker is already dead, and the dropdown is
objectui's own hardcoded copy of this enum. Tracked as objectui follow-up in
degrades to free text (strictly better than a picker that lists nothing).

Closes #3133. Supersedes #3120 and #3122, which only deleted the duplicated
docs callout without touching the name that causes the confusion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
os-zhuang added a commit that referenced this pull request Jul 17, 2026
…l` (#3137)

Closes #3133. Renames the last platform surface projecting the ADR-0090 D3 reserved word: `ApproverType.role` → `org_membership_level`, with `role` kept as a deprecated alias for one window (resolves identically, warns). Publishes `xEnumDeprecated` on the node configSchema so Studio drops the deprecated spelling from the approver-type picker. Supersedes #3120/#3122. objectui half: objectstack-ai/objectui#2643 (merged).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant